Skip to content

Conversation

bwendling
Copy link
Collaborator

@bwendling bwendling commented Dec 18, 2023

There are many issues that popped up with the counted_by feature. The
patch #73730 has grown too large and approval is blocking Linux testing.

Includes reverts of:
commit 769bc11 ("[Clang] Implement the 'counted_by' attribute (#68750)")
commit bc09ec6 ("[CodeGen] Revamp counted_by calculations (#70606)")
commit 1a09cfb ("[Clang] counted_by attr can apply only to C99 flexible array members (#72347)")
commit a76adfb ("[NFC][Clang] Refactor code to calculate flexible array member size (#72790)")
commit d8447c7 ("[Clang] Correct handling of negative and out-of-bounds indices (#71877)")
Partial commit b31cd07 ("[Clang] Regenerate test checks (NFC)")

Closes #73168
Closes #75173

There are many issues that popped up with the counted_by feature. The
patch has grown too large and approval is blocking Linux testing.

Includes reverts of:
commit 769bc11 ("[Clang] Implement the 'counted_by' attribute (llvm#68750)")
commit bc09ec6 ("[CodeGen] Revamp counted_by calculations (llvm#70606)")
commit 1a09cfb ("[Clang] counted_by attr can apply only to C99 flexible array members (llvm#72347)")
commit a76adfb ("[NFC][Clang] Refactor code to calculate flexible array member size (llvm#72790)")
commit d8447c7 ("[Clang] Correct handling of negative and out-of-bounds indices (llvm#71877)")
Partial commit b31cd07 ("[Clang] Regenerate test checks (NFC)")
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Dec 18, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2023

@llvm/pr-subscribers-clang-codegen

Author: Bill Wendling (bwendling)

Changes

There are many issues that popped up with the counted_by feature. The patch has grown too large and approval is blocking Linux testing.

Includes reverts of:
commit 769bc11 ("[Clang] Implement the 'counted_by' attribute (#68750)") commit bc09ec6 ("[CodeGen] Revamp counted_by calculations (#70606)") commit 1a09cfb ("[Clang] counted_by attr can apply only to C99 flexible array members (#72347)") commit a76adfb ("[NFC][Clang] Refactor code to calculate flexible array member size (#72790)") commit d8447c7 ("[Clang] Correct handling of negative and out-of-bounds indices (#71877)") Partial commit b31cd07 ("[Clang] Regenerate test checks (NFC)")


Patch is 101.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/75857.diff

21 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (-5)
  • (modified) clang/include/clang/AST/Decl.h (-24)
  • (modified) clang/include/clang/AST/DeclBase.h (-10)
  • (modified) clang/include/clang/Basic/Attr.td (-18)
  • (modified) clang/include/clang/Basic/AttrDocs.td (-66)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (-11)
  • (modified) clang/include/clang/Sema/Sema.h (-3)
  • (modified) clang/include/clang/Sema/TypoCorrection.h (+4-8)
  • (modified) clang/lib/AST/ASTImporter.cpp (-13)
  • (modified) clang/lib/AST/DeclBase.cpp (+1-73)
  • (modified) clang/lib/AST/Expr.cpp (+73-10)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (-167)
  • (modified) clang/lib/CodeGen/CGExpr.cpp (+3-126)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (-16)
  • (modified) clang/lib/Sema/SemaDecl.cpp (-14)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (-90)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+7-9)
  • (removed) clang/test/CodeGen/attr-counted-by.c (-742)
  • (modified) clang/test/CodeGen/bounds-checking.c (+1-9)
  • (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (-1)
  • (removed) clang/test/Sema/attr-counted-by.c (-55)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2e32f8b36d23de..edb97347f07716 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,11 +199,6 @@ C Language Changes
 - ``structs``, ``unions``, and ``arrays`` that are const may now be used as
   constant expressions.  This change is more consistent with the behavior of
   GCC.
-- Clang now supports the C-only attribute ``counted_by``. When applied to a
-  struct's flexible array member, it points to the struct field that holds the
-  number of elements in the flexible array member. This information can improve
-  the results of the array bound sanitizer and the
-  ``__builtin_dynamic_object_size`` builtin.
 - Enums will now be represented in TBAA metadata using their actual underlying
   integer type. Previously they were treated as chars, which meant they could
   alias with all other types.
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index cd0878d7082514..f9bf9cc5de7cb4 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4332,30 +4332,6 @@ class RecordDecl : public TagDecl {
     return field_begin() == field_end();
   }
 
-  FieldDecl *getLastField() {
-    FieldDecl *FD = nullptr;
-    for (FieldDecl *Field : fields())
-      FD = Field;
-    return FD;
-  }
-  const FieldDecl *getLastField() const {
-    return const_cast<RecordDecl *>(this)->getLastField();
-  }
-
-  template <typename Functor>
-  const FieldDecl *findFieldIf(Functor &Pred) const {
-    for (const Decl *D : decls()) {
-      if (const auto *FD = dyn_cast<FieldDecl>(D); FD && Pred(FD))
-        return FD;
-
-      if (const auto *RD = dyn_cast<RecordDecl>(D))
-        if (const FieldDecl *FD = RD->findFieldIf(Pred))
-          return FD;
-    }
-
-    return nullptr;
-  }
-
   /// Note that the definition of this type is now complete.
   virtual void completeDefinition();
 
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 5b1038582bc674..10dcbdb262d84e 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -19,7 +19,6 @@
 #include "clang/AST/SelectorLocationsKind.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
-#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -489,15 +488,6 @@ class alignas(8) Decl {
   // Return true if this is a FileContext Decl.
   bool isFileContextDecl() const;
 
-  /// Whether it resembles a flexible array member. This is a static member
-  /// because we want to be able to call it with a nullptr. That allows us to
-  /// perform non-Decl specific checks based on the object's type and strict
-  /// flex array level.
-  static bool isFlexibleArrayMemberLike(
-      ASTContext &Context, const Decl *D, QualType Ty,
-      LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
-      bool IgnoreTemplateOrMacroSubstitution);
-
   ASTContext &getASTContext() const LLVM_READONLY;
 
   /// Helper to get the language options from the ASTContext.
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 2b57058d3f1c75..db17211747b17d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4331,24 +4331,6 @@ def AvailableOnlyInDefaultEvalMethod : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def CountedBy : InheritableAttr {
-  let Spellings = [Clang<"counted_by">];
-  let Subjects = SubjectList<[Field]>;
-  let Args = [IdentifierArgument<"CountedByField">];
-  let Documentation = [CountedByDocs];
-  let LangOpts = [COnly];
-  // FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
-  // isn't yet available due to the fact that we're still parsing the
-  // structure. Maybe that code could be changed sometime in the future.
-  code AdditionalMembers = [{
-    private:
-      SourceRange CountedByFieldLoc;
-    public:
-      SourceRange getCountedByFieldLoc() const { return CountedByFieldLoc; }
-      void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; }
-  }];
-}
-
 def PreferredType: InheritableAttr {
   let Spellings = [Clang<"preferred_type">];
   let Subjects = SubjectList<[BitField], ErrorDiag>;
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 90041fa8dbb30b..98a7ecc7fd7df3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7500,72 +7500,6 @@ attribute, they default to the value ``65535``.
 }];
 }
 
-def CountedByDocs : Documentation {
-  let Category = DocCatField;
-  let Content = [{
-Clang supports the ``counted_by`` attribute on the flexible array member of a
-structure in C. The argument for the attribute is the name of a field member in
-the same structure holding the count of elements in the flexible array. This
-information can be used to improve the results of the array bound sanitizer and
-the ``__builtin_dynamic_object_size`` builtin.
-
-For example, the following code:
-
-.. code-block:: c
-
-  struct bar;
-
-  struct foo {
-    size_t count;
-    char other;
-    struct bar *array[] __attribute__((counted_by(count)));
-  };
-
-specifies that the flexible array member ``array`` has the number of elements
-allocated for it stored in ``count``. This establishes a relationship between
-``array`` and ``count``. Specifically, ``p->array`` must have at least
-``p->count`` number of elements available. It's the user's responsibility to
-ensure that this relationship is maintained through changes to the structure.
-
-In the following example, the allocated array erroneously has fewer elements
-than what's specified by ``p->count``. This would result in an out-of-bounds
-access not being detected.
-
-.. code-block:: c
-
-  #define SIZE_INCR 42
-
-  struct foo *p;
-
-  void foo_alloc(size_t count) {
-    p = malloc(MAX(sizeof(struct foo),
-                   offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
-    p->count = count + SIZE_INCR;
-  }
-
-The next example updates ``p->count``, breaking the relationship requirement
-that ``p->array`` must have at least ``p->count`` number of elements available:
-
-.. code-block:: c
-
-  #define SIZE_INCR 42
-
-  struct foo *p;
-
-  void foo_alloc(size_t count) {
-    p = malloc(MAX(sizeof(struct foo),
-                   offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
-    p->count = count;
-  }
-
-  void use_foo(int index) {
-    p->count += SIZE_INCR + 1; /* 'count' is now larger than the number of elements of 'array'. */
-    p->array[index] = 0;       /* the sanitizer can't properly check if this is an out-of-bounds access. */
-  }
-
-  }];
-}
-
 def CoroOnlyDestroyWhenCompleteDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6e6f56ff75e5f9..c100041ca400f2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6429,17 +6429,6 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
   "field %0 can overwrite instance variable %1 with variable sized type %2"
   " in superclass %3">, InGroup<ObjCFlexibleArray>;
 
-def err_counted_by_attr_not_on_flexible_array_member : Error<
-  "'counted_by' only applies to C99 flexible array members">;
-def err_counted_by_attr_refers_to_flexible_array : Error<
-  "'counted_by' cannot refer to the flexible array %0">;
-def err_counted_by_must_be_in_structure : Error<
-  "field %0 in 'counted_by' not inside structure">;
-def err_flexible_array_counted_by_attr_field_not_integer : Error<
-  "field %0 in 'counted_by' must be a non-boolean integer type">;
-def note_flexible_array_counted_by_attr_field : Note<
-  "field %0 declared here">;
-
 let CategoryName = "ARC Semantic Issue" in {
 
 // ARC-mode diagnostics.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a4f8fc1845b1ce..9887cc4ba4658d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4799,8 +4799,6 @@ class Sema final {
   bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
                              const AttributeCommonInfo &A);
 
-  bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);
-
   /// Adjust the calling convention of a method to be the ABI default if it
   /// wasn't specified explicitly.  This handles method types formed from
   /// function type typedefs and typename template arguments.
@@ -5644,7 +5642,6 @@ class Sema final {
                       CorrectionCandidateCallback &CCC,
                       TemplateArgumentListInfo *ExplicitTemplateArgs = nullptr,
                       ArrayRef<Expr *> Args = std::nullopt,
-                      DeclContext *LookupCtx = nullptr,
                       TypoExpr **Out = nullptr);
 
   DeclResult LookupIvarInObjCMethod(LookupResult &Lookup, Scope *S,
diff --git a/clang/include/clang/Sema/TypoCorrection.h b/clang/include/clang/Sema/TypoCorrection.h
index 09de164297e7ba..e0f8d152dbe554 100644
--- a/clang/include/clang/Sema/TypoCorrection.h
+++ b/clang/include/clang/Sema/TypoCorrection.h
@@ -282,7 +282,7 @@ class CorrectionCandidateCallback {
 public:
   static const unsigned InvalidDistance = TypoCorrection::InvalidDistance;
 
-  explicit CorrectionCandidateCallback(const IdentifierInfo *Typo = nullptr,
+  explicit CorrectionCandidateCallback(IdentifierInfo *Typo = nullptr,
                                        NestedNameSpecifier *TypoNNS = nullptr)
       : Typo(Typo), TypoNNS(TypoNNS) {}
 
@@ -319,7 +319,7 @@ class CorrectionCandidateCallback {
   /// this method.
   virtual std::unique_ptr<CorrectionCandidateCallback> clone() = 0;
 
-  void setTypoName(const IdentifierInfo *II) { Typo = II; }
+  void setTypoName(IdentifierInfo *II) { Typo = II; }
   void setTypoNNS(NestedNameSpecifier *NNS) { TypoNNS = NNS; }
 
   // Flags for context-dependent keywords. WantFunctionLikeCasts is only
@@ -345,13 +345,13 @@ class CorrectionCandidateCallback {
            candidate.getCorrectionSpecifier() == TypoNNS;
   }
 
-  const IdentifierInfo *Typo;
+  IdentifierInfo *Typo;
   NestedNameSpecifier *TypoNNS;
 };
 
 class DefaultFilterCCC final : public CorrectionCandidateCallback {
 public:
-  explicit DefaultFilterCCC(const IdentifierInfo *Typo = nullptr,
+  explicit DefaultFilterCCC(IdentifierInfo *Typo = nullptr,
                             NestedNameSpecifier *TypoNNS = nullptr)
       : CorrectionCandidateCallback(Typo, TypoNNS) {}
 
@@ -365,10 +365,6 @@ class DefaultFilterCCC final : public CorrectionCandidateCallback {
 template <class C>
 class DeclFilterCCC final : public CorrectionCandidateCallback {
 public:
-  explicit DeclFilterCCC(const IdentifierInfo *Typo = nullptr,
-                         NestedNameSpecifier *TypoNNS = nullptr)
-      : CorrectionCandidateCallback(Typo, TypoNNS) {}
-
   bool ValidateCandidate(const TypoCorrection &candidate) override {
     return candidate.getCorrectionDeclAs<C>();
   }
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..49d0dd218d6830 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -9003,10 +9003,6 @@ class AttrImporter {
 public:
   AttrImporter(ASTImporter &I) : Importer(I), NImporter(I) {}
 
-  // Useful for accessing the imported attribute.
-  template <typename T> T *castAttrAs() { return cast<T>(ToAttr); }
-  template <typename T> const T *castAttrAs() const { return cast<T>(ToAttr); }
-
   // Create an "importer" for an attribute parameter.
   // Result of the 'value()' of that object is to be passed to the function
   // 'importAttr', in the order that is expected by the attribute class.
@@ -9214,15 +9210,6 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
                   From->args_size());
     break;
   }
-  case attr::CountedBy: {
-    AI.cloneAttr(FromAttr);
-    const auto *CBA = cast<CountedByAttr>(FromAttr);
-    Expected<SourceRange> SR = Import(CBA->getCountedByFieldLoc()).get();
-    if (!SR)
-      return SR.takeError();
-    AI.castAttrAs<CountedByAttr>()->setCountedByFieldLoc(SR.get());
-    break;
-  }
 
   default: {
     // The default branch works for attributes that have no arguments to import.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index e4d7169752bc85..5e03f0223d311c 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -29,6 +29,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/PartialDiagnostic.h"
@@ -410,79 +411,6 @@ bool Decl::isFileContextDecl() const {
   return DC && DC->isFileContext();
 }
 
-bool Decl::isFlexibleArrayMemberLike(
-    ASTContext &Ctx, const Decl *D, QualType Ty,
-    LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
-    bool IgnoreTemplateOrMacroSubstitution) {
-  // For compatibility with existing code, we treat arrays of length 0 or
-  // 1 as flexible array members.
-  const auto *CAT = Ctx.getAsConstantArrayType(Ty);
-  if (CAT) {
-    using FAMKind = LangOptions::StrictFlexArraysLevelKind;
-
-    llvm::APInt Size = CAT->getSize();
-    if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
-      return false;
-
-    // GCC extension, only allowed to represent a FAM.
-    if (Size.isZero())
-      return true;
-
-    if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
-      return false;
-
-    if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
-      return false;
-  } else if (!Ctx.getAsIncompleteArrayType(Ty)) {
-    return false;
-  }
-
-  if (const auto *OID = dyn_cast_if_present<ObjCIvarDecl>(D))
-    return OID->getNextIvar() == nullptr;
-
-  const auto *FD = dyn_cast_if_present<FieldDecl>(D);
-  if (!FD)
-    return false;
-
-  if (CAT) {
-    // GCC treats an array memeber of a union as an FAM if the size is one or
-    // zero.
-    llvm::APInt Size = CAT->getSize();
-    if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
-      return true;
-  }
-
-  // Don't consider sizes resulting from macro expansions or template argument
-  // substitution to form C89 tail-padded arrays.
-  if (IgnoreTemplateOrMacroSubstitution) {
-    TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
-    while (TInfo) {
-      TypeLoc TL = TInfo->getTypeLoc();
-
-      // Look through typedefs.
-      if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
-        const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
-        TInfo = TDL->getTypeSourceInfo();
-        continue;
-      }
-
-      if (auto CTL = TL.getAs<ConstantArrayTypeLoc>()) {
-        if (const Expr *SizeExpr =
-                dyn_cast_if_present<IntegerLiteral>(CTL.getSizeExpr());
-            !SizeExpr || SizeExpr->getExprLoc().isMacroID())
-          return false;
-      }
-
-      break;
-    }
-  }
-
-  // Test that the field is the last in the structure.
-  RecordDecl::field_iterator FI(
-      DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
-  return ++FI == FD->getParent()->field_end();
-}
-
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
   if (auto *TUD = dyn_cast<TranslationUnitDecl>(this))
     return TUD;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index b125fc676da841..a90f92d07f86d2 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -205,22 +205,85 @@ bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
 }
 
 bool Expr::isFlexibleArrayMemberLike(
-    ASTContext &Ctx,
+    ASTContext &Context,
     LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
     bool IgnoreTemplateOrMacroSubstitution) const {
+
+  // For compatibility with existing code, we treat arrays of length 0 or
+  // 1 as flexible array members.
+  const auto *CAT = Context.getAsConstantArrayType(getType());
+  if (CAT) {
+    llvm::APInt Size = CAT->getSize();
+
+    using FAMKind = LangOptions::StrictFlexArraysLevelKind;
+
+    if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
+      return false;
+
+    // GCC extension, only allowed to represent a FAM.
+    if (Size == 0)
+      return true;
+
+    if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
+      return false;
+
+    if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
+      return false;
+  } else if (!Context.getAsIncompleteArrayType(getType()))
+    return false;
+
   const Expr *E = IgnoreParens();
-  const Decl *D = nullptr;
 
-  if (const auto *ME = dyn_cast<MemberExpr>(E))
-    D = ME->getMemberDecl();
-  else if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
-    D = DRE->getDecl();
+  const NamedDecl *ND = nullptr;
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
+    ND = DRE->getDecl();
+  else if (const auto *ME = dyn_cast<MemberExpr>(E))
+    ND = ME->getMemberDecl();
   else if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(E))
-    D = IRE->getDecl();
+    return IRE->getDecl()->getNextIvar() == nullptr;
+
+  if (!ND)
+    return false;
 
-  return Decl::isFlexibleArrayMemberLike(Ctx, D, E->getType(),
-                                         StrictFlexArraysLevel,
-                                         IgnoreTemplateOrMacroSubstitution);
+  // A flexible array member must be the last member in the class.
+  // FIXME: If the base type of the member expr is not FD->getParent(),
+  // this should not be treated as a flexible array member access.
+  if (const auto *FD = dyn_cast<FieldDecl>(ND)) {
+    // GCC treats an array memeber of a union as an FAM if the size is one or
+    // zero.
+    if (CAT) {
+      llvm::APInt Size = CAT->getSize();
+      if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
+        return true;
+    }
+
+    // Don't consider sizes resulting from macro expansions or template argument
+    // substitution to form C89 tail-padded arrays.
+    if (IgnoreTemplateOrMacroSubstitution) {
+      TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
+      while (TInfo) {
+        TypeLoc TL = TInfo->getTypeLoc();
+        // Look through typedefs.
+        if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
+          const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
+          TInfo = TDL->getTypeSourceInfo();
+          continue;
+        }
+        if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
+          const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
+          if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
+            return false;
+        }
+        break;
+      }
+    }
+
+    RecordDecl::field_iterator FI(
+        DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
+    return ++FI == FD->getParent()->field_end();
+  }
+
+  return false;
 }
 
 const ValueDecl *
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4eb1686f095062..a29304c81928cc 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -25,7 +25,6 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/OSLog.h"
-#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
@@ -819,165 +818,6 @@ CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type,
   return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true);
 }
 
-llvm::Value *
-CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
-                                             llvm::IntegerType *ResType) {
-  // The code generated here calculates the size of a struct with a flexible
-  // array member that uses the counted_by attribute. There are two instances
-  // we handle:
-  //
-  //       struct s {
-  //         unsigned long flags;
-  //         int count;
-  //         int array[] __attribute__((counted_by(count)));
-  //       }
-  //
-  //   1) bdos of the flexible array itself:
-  //
-  //     __builtin_dynamic_object_size(p->array, 1) ==
-  //         p->count * sizeof(*p->...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2023

@llvm/pr-subscribers-clang

Author: Bill Wendling (bwendling)

Changes

There are many issues that popped up with the counted_by feature. The patch has grown too large and approval is blocking Linux testing.

Includes reverts of:
commit 769bc11 ("[Clang] Implement the 'counted_by' attribute (#68750)") commit bc09ec6 ("[CodeGen] Revamp counted_by calculations (#70606)") commit 1a09cfb ("[Clang] counted_by attr can apply only to C99 flexible array members (#72347)") commit a76adfb ("[NFC][Clang] Refactor code to calculate flexible array member size (#72790)") commit d8447c7 ("[Clang] Correct handling of negative and out-of-bounds indices (#71877)") Partial commit b31cd07 ("[Clang] Regenerate test checks (NFC)")


Patch is 101.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/75857.diff

21 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (-5)
  • (modified) clang/include/clang/AST/Decl.h (-24)
  • (modified) clang/include/clang/AST/DeclBase.h (-10)
  • (modified) clang/include/clang/Basic/Attr.td (-18)
  • (modified) clang/include/clang/Basic/AttrDocs.td (-66)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (-11)
  • (modified) clang/include/clang/Sema/Sema.h (-3)
  • (modified) clang/include/clang/Sema/TypoCorrection.h (+4-8)
  • (modified) clang/lib/AST/ASTImporter.cpp (-13)
  • (modified) clang/lib/AST/DeclBase.cpp (+1-73)
  • (modified) clang/lib/AST/Expr.cpp (+73-10)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (-167)
  • (modified) clang/lib/CodeGen/CGExpr.cpp (+3-126)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (-16)
  • (modified) clang/lib/Sema/SemaDecl.cpp (-14)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (-90)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+7-9)
  • (removed) clang/test/CodeGen/attr-counted-by.c (-742)
  • (modified) clang/test/CodeGen/bounds-checking.c (+1-9)
  • (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (-1)
  • (removed) clang/test/Sema/attr-counted-by.c (-55)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2e32f8b36d23de..edb97347f07716 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,11 +199,6 @@ C Language Changes
 - ``structs``, ``unions``, and ``arrays`` that are const may now be used as
   constant expressions.  This change is more consistent with the behavior of
   GCC.
-- Clang now supports the C-only attribute ``counted_by``. When applied to a
-  struct's flexible array member, it points to the struct field that holds the
-  number of elements in the flexible array member. This information can improve
-  the results of the array bound sanitizer and the
-  ``__builtin_dynamic_object_size`` builtin.
 - Enums will now be represented in TBAA metadata using their actual underlying
   integer type. Previously they were treated as chars, which meant they could
   alias with all other types.
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index cd0878d7082514..f9bf9cc5de7cb4 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4332,30 +4332,6 @@ class RecordDecl : public TagDecl {
     return field_begin() == field_end();
   }
 
-  FieldDecl *getLastField() {
-    FieldDecl *FD = nullptr;
-    for (FieldDecl *Field : fields())
-      FD = Field;
-    return FD;
-  }
-  const FieldDecl *getLastField() const {
-    return const_cast<RecordDecl *>(this)->getLastField();
-  }
-
-  template <typename Functor>
-  const FieldDecl *findFieldIf(Functor &Pred) const {
-    for (const Decl *D : decls()) {
-      if (const auto *FD = dyn_cast<FieldDecl>(D); FD && Pred(FD))
-        return FD;
-
-      if (const auto *RD = dyn_cast<RecordDecl>(D))
-        if (const FieldDecl *FD = RD->findFieldIf(Pred))
-          return FD;
-    }
-
-    return nullptr;
-  }
-
   /// Note that the definition of this type is now complete.
   virtual void completeDefinition();
 
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 5b1038582bc674..10dcbdb262d84e 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -19,7 +19,6 @@
 #include "clang/AST/SelectorLocationsKind.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
-#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -489,15 +488,6 @@ class alignas(8) Decl {
   // Return true if this is a FileContext Decl.
   bool isFileContextDecl() const;
 
-  /// Whether it resembles a flexible array member. This is a static member
-  /// because we want to be able to call it with a nullptr. That allows us to
-  /// perform non-Decl specific checks based on the object's type and strict
-  /// flex array level.
-  static bool isFlexibleArrayMemberLike(
-      ASTContext &Context, const Decl *D, QualType Ty,
-      LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
-      bool IgnoreTemplateOrMacroSubstitution);
-
   ASTContext &getASTContext() const LLVM_READONLY;
 
   /// Helper to get the language options from the ASTContext.
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 2b57058d3f1c75..db17211747b17d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4331,24 +4331,6 @@ def AvailableOnlyInDefaultEvalMethod : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def CountedBy : InheritableAttr {
-  let Spellings = [Clang<"counted_by">];
-  let Subjects = SubjectList<[Field]>;
-  let Args = [IdentifierArgument<"CountedByField">];
-  let Documentation = [CountedByDocs];
-  let LangOpts = [COnly];
-  // FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
-  // isn't yet available due to the fact that we're still parsing the
-  // structure. Maybe that code could be changed sometime in the future.
-  code AdditionalMembers = [{
-    private:
-      SourceRange CountedByFieldLoc;
-    public:
-      SourceRange getCountedByFieldLoc() const { return CountedByFieldLoc; }
-      void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; }
-  }];
-}
-
 def PreferredType: InheritableAttr {
   let Spellings = [Clang<"preferred_type">];
   let Subjects = SubjectList<[BitField], ErrorDiag>;
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 90041fa8dbb30b..98a7ecc7fd7df3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7500,72 +7500,6 @@ attribute, they default to the value ``65535``.
 }];
 }
 
-def CountedByDocs : Documentation {
-  let Category = DocCatField;
-  let Content = [{
-Clang supports the ``counted_by`` attribute on the flexible array member of a
-structure in C. The argument for the attribute is the name of a field member in
-the same structure holding the count of elements in the flexible array. This
-information can be used to improve the results of the array bound sanitizer and
-the ``__builtin_dynamic_object_size`` builtin.
-
-For example, the following code:
-
-.. code-block:: c
-
-  struct bar;
-
-  struct foo {
-    size_t count;
-    char other;
-    struct bar *array[] __attribute__((counted_by(count)));
-  };
-
-specifies that the flexible array member ``array`` has the number of elements
-allocated for it stored in ``count``. This establishes a relationship between
-``array`` and ``count``. Specifically, ``p->array`` must have at least
-``p->count`` number of elements available. It's the user's responsibility to
-ensure that this relationship is maintained through changes to the structure.
-
-In the following example, the allocated array erroneously has fewer elements
-than what's specified by ``p->count``. This would result in an out-of-bounds
-access not being detected.
-
-.. code-block:: c
-
-  #define SIZE_INCR 42
-
-  struct foo *p;
-
-  void foo_alloc(size_t count) {
-    p = malloc(MAX(sizeof(struct foo),
-                   offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
-    p->count = count + SIZE_INCR;
-  }
-
-The next example updates ``p->count``, breaking the relationship requirement
-that ``p->array`` must have at least ``p->count`` number of elements available:
-
-.. code-block:: c
-
-  #define SIZE_INCR 42
-
-  struct foo *p;
-
-  void foo_alloc(size_t count) {
-    p = malloc(MAX(sizeof(struct foo),
-                   offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
-    p->count = count;
-  }
-
-  void use_foo(int index) {
-    p->count += SIZE_INCR + 1; /* 'count' is now larger than the number of elements of 'array'. */
-    p->array[index] = 0;       /* the sanitizer can't properly check if this is an out-of-bounds access. */
-  }
-
-  }];
-}
-
 def CoroOnlyDestroyWhenCompleteDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6e6f56ff75e5f9..c100041ca400f2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6429,17 +6429,6 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
   "field %0 can overwrite instance variable %1 with variable sized type %2"
   " in superclass %3">, InGroup<ObjCFlexibleArray>;
 
-def err_counted_by_attr_not_on_flexible_array_member : Error<
-  "'counted_by' only applies to C99 flexible array members">;
-def err_counted_by_attr_refers_to_flexible_array : Error<
-  "'counted_by' cannot refer to the flexible array %0">;
-def err_counted_by_must_be_in_structure : Error<
-  "field %0 in 'counted_by' not inside structure">;
-def err_flexible_array_counted_by_attr_field_not_integer : Error<
-  "field %0 in 'counted_by' must be a non-boolean integer type">;
-def note_flexible_array_counted_by_attr_field : Note<
-  "field %0 declared here">;
-
 let CategoryName = "ARC Semantic Issue" in {
 
 // ARC-mode diagnostics.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a4f8fc1845b1ce..9887cc4ba4658d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4799,8 +4799,6 @@ class Sema final {
   bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
                              const AttributeCommonInfo &A);
 
-  bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);
-
   /// Adjust the calling convention of a method to be the ABI default if it
   /// wasn't specified explicitly.  This handles method types formed from
   /// function type typedefs and typename template arguments.
@@ -5644,7 +5642,6 @@ class Sema final {
                       CorrectionCandidateCallback &CCC,
                       TemplateArgumentListInfo *ExplicitTemplateArgs = nullptr,
                       ArrayRef<Expr *> Args = std::nullopt,
-                      DeclContext *LookupCtx = nullptr,
                       TypoExpr **Out = nullptr);
 
   DeclResult LookupIvarInObjCMethod(LookupResult &Lookup, Scope *S,
diff --git a/clang/include/clang/Sema/TypoCorrection.h b/clang/include/clang/Sema/TypoCorrection.h
index 09de164297e7ba..e0f8d152dbe554 100644
--- a/clang/include/clang/Sema/TypoCorrection.h
+++ b/clang/include/clang/Sema/TypoCorrection.h
@@ -282,7 +282,7 @@ class CorrectionCandidateCallback {
 public:
   static const unsigned InvalidDistance = TypoCorrection::InvalidDistance;
 
-  explicit CorrectionCandidateCallback(const IdentifierInfo *Typo = nullptr,
+  explicit CorrectionCandidateCallback(IdentifierInfo *Typo = nullptr,
                                        NestedNameSpecifier *TypoNNS = nullptr)
       : Typo(Typo), TypoNNS(TypoNNS) {}
 
@@ -319,7 +319,7 @@ class CorrectionCandidateCallback {
   /// this method.
   virtual std::unique_ptr<CorrectionCandidateCallback> clone() = 0;
 
-  void setTypoName(const IdentifierInfo *II) { Typo = II; }
+  void setTypoName(IdentifierInfo *II) { Typo = II; }
   void setTypoNNS(NestedNameSpecifier *NNS) { TypoNNS = NNS; }
 
   // Flags for context-dependent keywords. WantFunctionLikeCasts is only
@@ -345,13 +345,13 @@ class CorrectionCandidateCallback {
            candidate.getCorrectionSpecifier() == TypoNNS;
   }
 
-  const IdentifierInfo *Typo;
+  IdentifierInfo *Typo;
   NestedNameSpecifier *TypoNNS;
 };
 
 class DefaultFilterCCC final : public CorrectionCandidateCallback {
 public:
-  explicit DefaultFilterCCC(const IdentifierInfo *Typo = nullptr,
+  explicit DefaultFilterCCC(IdentifierInfo *Typo = nullptr,
                             NestedNameSpecifier *TypoNNS = nullptr)
       : CorrectionCandidateCallback(Typo, TypoNNS) {}
 
@@ -365,10 +365,6 @@ class DefaultFilterCCC final : public CorrectionCandidateCallback {
 template <class C>
 class DeclFilterCCC final : public CorrectionCandidateCallback {
 public:
-  explicit DeclFilterCCC(const IdentifierInfo *Typo = nullptr,
-                         NestedNameSpecifier *TypoNNS = nullptr)
-      : CorrectionCandidateCallback(Typo, TypoNNS) {}
-
   bool ValidateCandidate(const TypoCorrection &candidate) override {
     return candidate.getCorrectionDeclAs<C>();
   }
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index f1f335118f37a4..49d0dd218d6830 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -9003,10 +9003,6 @@ class AttrImporter {
 public:
   AttrImporter(ASTImporter &I) : Importer(I), NImporter(I) {}
 
-  // Useful for accessing the imported attribute.
-  template <typename T> T *castAttrAs() { return cast<T>(ToAttr); }
-  template <typename T> const T *castAttrAs() const { return cast<T>(ToAttr); }
-
   // Create an "importer" for an attribute parameter.
   // Result of the 'value()' of that object is to be passed to the function
   // 'importAttr', in the order that is expected by the attribute class.
@@ -9214,15 +9210,6 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
                   From->args_size());
     break;
   }
-  case attr::CountedBy: {
-    AI.cloneAttr(FromAttr);
-    const auto *CBA = cast<CountedByAttr>(FromAttr);
-    Expected<SourceRange> SR = Import(CBA->getCountedByFieldLoc()).get();
-    if (!SR)
-      return SR.takeError();
-    AI.castAttrAs<CountedByAttr>()->setCountedByFieldLoc(SR.get());
-    break;
-  }
 
   default: {
     // The default branch works for attributes that have no arguments to import.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index e4d7169752bc85..5e03f0223d311c 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -29,6 +29,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/PartialDiagnostic.h"
@@ -410,79 +411,6 @@ bool Decl::isFileContextDecl() const {
   return DC && DC->isFileContext();
 }
 
-bool Decl::isFlexibleArrayMemberLike(
-    ASTContext &Ctx, const Decl *D, QualType Ty,
-    LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
-    bool IgnoreTemplateOrMacroSubstitution) {
-  // For compatibility with existing code, we treat arrays of length 0 or
-  // 1 as flexible array members.
-  const auto *CAT = Ctx.getAsConstantArrayType(Ty);
-  if (CAT) {
-    using FAMKind = LangOptions::StrictFlexArraysLevelKind;
-
-    llvm::APInt Size = CAT->getSize();
-    if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
-      return false;
-
-    // GCC extension, only allowed to represent a FAM.
-    if (Size.isZero())
-      return true;
-
-    if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
-      return false;
-
-    if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
-      return false;
-  } else if (!Ctx.getAsIncompleteArrayType(Ty)) {
-    return false;
-  }
-
-  if (const auto *OID = dyn_cast_if_present<ObjCIvarDecl>(D))
-    return OID->getNextIvar() == nullptr;
-
-  const auto *FD = dyn_cast_if_present<FieldDecl>(D);
-  if (!FD)
-    return false;
-
-  if (CAT) {
-    // GCC treats an array memeber of a union as an FAM if the size is one or
-    // zero.
-    llvm::APInt Size = CAT->getSize();
-    if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
-      return true;
-  }
-
-  // Don't consider sizes resulting from macro expansions or template argument
-  // substitution to form C89 tail-padded arrays.
-  if (IgnoreTemplateOrMacroSubstitution) {
-    TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
-    while (TInfo) {
-      TypeLoc TL = TInfo->getTypeLoc();
-
-      // Look through typedefs.
-      if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
-        const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
-        TInfo = TDL->getTypeSourceInfo();
-        continue;
-      }
-
-      if (auto CTL = TL.getAs<ConstantArrayTypeLoc>()) {
-        if (const Expr *SizeExpr =
-                dyn_cast_if_present<IntegerLiteral>(CTL.getSizeExpr());
-            !SizeExpr || SizeExpr->getExprLoc().isMacroID())
-          return false;
-      }
-
-      break;
-    }
-  }
-
-  // Test that the field is the last in the structure.
-  RecordDecl::field_iterator FI(
-      DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
-  return ++FI == FD->getParent()->field_end();
-}
-
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
   if (auto *TUD = dyn_cast<TranslationUnitDecl>(this))
     return TUD;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index b125fc676da841..a90f92d07f86d2 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -205,22 +205,85 @@ bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
 }
 
 bool Expr::isFlexibleArrayMemberLike(
-    ASTContext &Ctx,
+    ASTContext &Context,
     LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
     bool IgnoreTemplateOrMacroSubstitution) const {
+
+  // For compatibility with existing code, we treat arrays of length 0 or
+  // 1 as flexible array members.
+  const auto *CAT = Context.getAsConstantArrayType(getType());
+  if (CAT) {
+    llvm::APInt Size = CAT->getSize();
+
+    using FAMKind = LangOptions::StrictFlexArraysLevelKind;
+
+    if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
+      return false;
+
+    // GCC extension, only allowed to represent a FAM.
+    if (Size == 0)
+      return true;
+
+    if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
+      return false;
+
+    if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
+      return false;
+  } else if (!Context.getAsIncompleteArrayType(getType()))
+    return false;
+
   const Expr *E = IgnoreParens();
-  const Decl *D = nullptr;
 
-  if (const auto *ME = dyn_cast<MemberExpr>(E))
-    D = ME->getMemberDecl();
-  else if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
-    D = DRE->getDecl();
+  const NamedDecl *ND = nullptr;
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
+    ND = DRE->getDecl();
+  else if (const auto *ME = dyn_cast<MemberExpr>(E))
+    ND = ME->getMemberDecl();
   else if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(E))
-    D = IRE->getDecl();
+    return IRE->getDecl()->getNextIvar() == nullptr;
+
+  if (!ND)
+    return false;
 
-  return Decl::isFlexibleArrayMemberLike(Ctx, D, E->getType(),
-                                         StrictFlexArraysLevel,
-                                         IgnoreTemplateOrMacroSubstitution);
+  // A flexible array member must be the last member in the class.
+  // FIXME: If the base type of the member expr is not FD->getParent(),
+  // this should not be treated as a flexible array member access.
+  if (const auto *FD = dyn_cast<FieldDecl>(ND)) {
+    // GCC treats an array memeber of a union as an FAM if the size is one or
+    // zero.
+    if (CAT) {
+      llvm::APInt Size = CAT->getSize();
+      if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
+        return true;
+    }
+
+    // Don't consider sizes resulting from macro expansions or template argument
+    // substitution to form C89 tail-padded arrays.
+    if (IgnoreTemplateOrMacroSubstitution) {
+      TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
+      while (TInfo) {
+        TypeLoc TL = TInfo->getTypeLoc();
+        // Look through typedefs.
+        if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
+          const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
+          TInfo = TDL->getTypeSourceInfo();
+          continue;
+        }
+        if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
+          const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
+          if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
+            return false;
+        }
+        break;
+      }
+    }
+
+    RecordDecl::field_iterator FI(
+        DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
+    return ++FI == FD->getParent()->field_end();
+  }
+
+  return false;
 }
 
 const ValueDecl *
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4eb1686f095062..a29304c81928cc 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -25,7 +25,6 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/OSLog.h"
-#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
@@ -819,165 +818,6 @@ CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type,
   return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true);
 }
 
-llvm::Value *
-CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
-                                             llvm::IntegerType *ResType) {
-  // The code generated here calculates the size of a struct with a flexible
-  // array member that uses the counted_by attribute. There are two instances
-  // we handle:
-  //
-  //       struct s {
-  //         unsigned long flags;
-  //         int count;
-  //         int array[] __attribute__((counted_by(count)));
-  //       }
-  //
-  //   1) bdos of the flexible array itself:
-  //
-  //     __builtin_dynamic_object_size(p->array, 1) ==
-  //         p->count * sizeof(*p->...
[truncated]

@nickdesaulniers
Copy link
Member

@nathanchance can you test this please to verify it's unbreaking the linux kernel builds?

@nathanchance
Copy link
Member

@nathanchance can you test this please to verify it's unbreaking the linux kernel builds?

A quick initial test shows this resolves the two cases that I found in #73168. I can do a fuller set of builds if necessary but since this is just backing out of __attribute__((__counted_by__(...))) altogether, I don't expect there to be any other issues as a result of this change (other than just uncovering other unrelated breakages from not having consistent ToT LLVM builds for almost a month in CI).

@bwendling bwendling merged commit cca4d6c into llvm:main Dec 18, 2023
@bwendling bwendling deleted the counted-by-revert branch December 18, 2023 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
5 participants